Skip to content

Conversation

@kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented Oct 8, 2025

This PR uses a signal handler to cut off process_visit calls that take longer than a configurable limit. The cutoff is soft, in the sense that process_visit still has a chance to perform cleanup as if during a pod shutdown. This means there is a chance that a hangup will trigger a timeout, then another hangup on Butler export; this should be rare enough that we can ignore it.

@kfindeisen kfindeisen marked this pull request as draft October 10, 2025 20:51
@kfindeisen kfindeisen force-pushed the tickets/DM-51311 branch 2 times, most recently from 1dce37c to 035abdd Compare October 13, 2025 22:18
@kfindeisen kfindeisen marked this pull request as ready for review October 13, 2025 22:20
@kfindeisen kfindeisen requested a review from hsinfang October 15, 2025 23:01
Copy link
Collaborator

@hsinfang hsinfang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Now that activator.py is an importable module, and not an executable,
it should list the members intended to be imported elsewhere.
The GracefulShutdownInterrupt and generic error handlers were identical
except for one line. It's easier (if slightly slower?) to use an
explicit isinstance check than to maintain two different handlers.
This lets the worker be called with a timeout, with the timeout's
implementation details hidden from process_visit's clients.
Like GracefulShutdownInterrupt, TimeoutInterrupt is raised
asynchronously, may leave the program in an invalid state, and may be
caught prematurely.
The timeout uses an alarm and a signal handler to interrupt application
code at will. This implementation is fairly robust, but assumes that
nothing else (especially sleep functions) uses SIGALRM and has the
usual caveats of signal handlers raising exceptions.
While Logger.exception is a handy way of printing stack traces, any log
message can include them. Include the stack trace as part of critical
logs instead of in a separate message.
@kfindeisen kfindeisen merged commit 0f8e6bf into main Oct 17, 2025
11 checks passed
@kfindeisen kfindeisen deleted the tickets/DM-51311 branch October 17, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants